Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #887 +/- ##
============================================
+ Coverage 30.47% 30.67% +0.20%
- Complexity 6627 6638 +11
============================================
Files 292 292
Lines 26432 26487 +55
============================================
+ Hits 8056 8126 +70
+ Misses 18376 18361 -15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The new excluded-strings setting can bypass the documented “20 values / 128 chars” limits when users mix commas and newlines, because validation and runtime parsing split the input differently.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a “do not encode these contacts” option to the ContactsEncoder feature, allowing administrators to exclude specific emails/phones (and fragments like domains) from being obfuscated during content processing.
Changes:
- Introduces an exclusion list setting (
data__email_decoder_excluded_strings) and wires it into ContactsEncoder params. - Adds exclusion parsing + matching logic (including phone digit normalization and
mailto:/tel:normalization). - Updates settings UI (field + help text) and adds unit tests covering exclusion behavior.
File summaries
| File | Description |
|---|---|
| tests/ApbctWP/ContactsEncoder/TestContactsEncoder.php | Adds tests ensuring excluded strings keep emails/phones unencoded in typical “title-like” content. |
| tests/ApbctWP/ContactsEncoder/Exclusions/TestExclusionsService.php | Adds tests for parsing excluded strings and matching email/phone variants. |
| lib/Cleantalk/Common/Helper.php | Updates dnsResolve() PHPDoc return/param types. |
| lib/Cleantalk/Common/ContactsEncoder/Exclusions/ExclusionsService.php | Adds excluded-strings parsing and contact exclusion matching logic. |
| lib/Cleantalk/Common/ContactsEncoder/Dto/Params.php | Adds excluded_strings param to pass exclusion list into the encoder. |
| lib/Cleantalk/Common/ContactsEncoder/ContactsEncoder.php | Skips encoding for matches that are deemed excluded by ExclusionsService. |
| lib/Cleantalk/ApbctWP/State.php | Adds default WP setting for the new excluded-strings textarea. |
| lib/Cleantalk/ApbctWP/PluginSettingsPage/SettingsField.php | Adjusts textarea title rendering to add a modifier class when help is present. |
| lib/Cleantalk/ApbctWP/ContactsEncoder/ContactsEncoder.php | Adds description/long-description strings for the new setting UI. |
| inc/cleantalk-settings.php | Registers the new setting, validates/sanitizes it, and adds long-description metadata. |
| css/src/cleantalk-admin-settings-page.css | Adds styling for the textarea title when help icon is present. |
| css/cleantalk-admin-settings-page.min.css | Minified CSS update reflecting the new modifier class styling. |
| cleantalk.php | Passes parsed excluded strings into ContactsEncoder params during instantiation. |
Review details
Files not reviewed (1)
- css/cleantalk-admin-settings-page.min.css: Generated file
- Files reviewed: 12/13 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The change is cohesive, covered by new tests, and integrates the new setting end-to-end with no confirmed functional regressions.
Review details
Files not reviewed (1)
- css/cleantalk-admin-settings-page.min.css: Generated file
- Files reviewed: 13/14 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The feature is implemented end-to-end (settings, sanitization, runtime behavior) with targeted tests, and only minor documentation clarity feedback remains.
Review details
Files not reviewed (1)
- css/cleantalk-admin-settings-page.min.css: Generated file
- Files reviewed: 13/14 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The new code introduces a UTF-8 truncation risk in excluded-string sanitization, and the touched dnsResolve() implementation appears incompatible with common hostname inputs (breaking its primary call site).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- css/cleantalk-admin-settings-page.min.css: Generated file
- Files reviewed: 13/14 changed files
- Comments generated: 3
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Exclusion matching currently applies digit-based phone comparisons to email matches too, which can incorrectly skip encoding for emails containing long digit sequences.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- css/cleantalk-admin-settings-page.min.css: Generated file
- Files reviewed: 13/14 changed files
- Comments generated: 2
- Review effort level: Lite
task https://app.doboard.com/1/task/44985